Capture handler refs at config construction rather than transport#2837
Open
laghee wants to merge 1 commit into
Open
Capture handler refs at config construction rather than transport#2837laghee wants to merge 1 commit into
laghee wants to merge 1 commit into
Conversation
…ransport construction WebkitMessagingTransport previously captured handler references in its own constructor. Since ContentFeature.messaging is lazy, transport construction can happen after ContentFeature init has run, which includes after apiManipulation has applied any prototype-side messageHandlers-nullifying patches. Constructor then reads undefined from window.webkit.messageHandlers and throws MissingHandler. Move the snapshot into WebkitMessagingConfig's constructor. Entry points construct the config eagerly at C-S-S bootstrap, before feature init runs, so the snapshot pre-dates any in-C-S-S apiManipulation patch. Transport constructor reads config.capturedHandlers rather than re-reading window.webkit.messageHandlers. Adds a regression test: construct config, nullify window.webkit.messageHandlers, construct transport, send message — send goes through the snapshot and reaches the original handler.
Contributor
|
This PR requires a manual review and approval from a member of one of the following teams:
|
Contributor
[Beta] Generated file diffTime updated: Fri, 03 Jul 2026 16:29:48 GMT Android
File has changed Apple
File has changed Chrome-mv3File has changed FirefoxFile has changed Integration
File has changed Windows
File has changed |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Injected PR Evaluation: Web Compatibility & Security
PR moves WebKit handler snapshot from lazy WebkitMessagingTransport construction to eager WebkitMessagingConfig construction at C-S-S bootstrap. This fixes a production bug where apiManipulation can nullify window.webkit.messageHandlers during feature init() before any feature first accesses ContentFeature.messaging, causing transport construction to read an absent namespace and throw MissingHandler.
Verified locally: all 9 specs in messaging-webkit.spec.js pass.
Web Compatibility Assessment
| File | Lines | Severity | Finding |
|---|---|---|---|
messaging/lib/webkit.js |
40–46, 184–195 | info | No page-visible API surface is modified. wkSend dispatch semantics are unchanged (ReflectApply on captured handler pair). This is an internal messaging reliability fix. |
messaging/lib/webkit.js |
164–170 | info | Positive: Snapshot at bootstrap (before load()/init() in apple.js) restores native messaging on sites where in-C-S-S hardening nullifies messageHandlers before lazy transport construction. Without this, features that defer this.messaging access would silently fail to reach native. |
injected/entry-points/apple.js |
18–24 | info | Bootstrap ordering (WebkitMessagingConfig → load() → init()) matches the fix contract. No ordering regression expected on Apple platforms. |
special-pages/shared/create-special-page-messaging.js |
38–41 | info | Special pages construct config and Messaging synchronously in the same call; no lazy gap. Behavior unchanged but consistent with the new model. |
No web-compat errors or warnings identified.
Security Assessment
| File | Lines | Severity | Finding |
|---|---|---|---|
messaging/lib/webkit.js |
57–64 | info | Send path unchanged: ReflectApply(captured.postMessage, captured.handler, [data]) avoids page-mutable .call/.bind. |
messaging/lib/webkit.js |
187–188 | info | Null-prototype cache preserved via { __proto__: null } literal (not Object.create(null)), resisting Object.create tampering at lazy-construction time. Existing unit test still covers this. |
messaging/lib/webkit.js |
184–195 | info | _snapshotWebkitHandlers reads window.webkit.messageHandlers via uncaptured window — same pattern as the removed captureWebkitHandlers. Acceptable because config is constructed at C-S-S bootstrap before page scripts and feature init; earlier snapshot actually narrows the tampering window vs deferred transport capture. |
messaging/lib/webkit.js |
43–46 | info | capturedHandlers === null guard fails fast at transport construction. An empty-but-truthy snapshot ({ __proto__: null } with no entries) still creates transport and fails per-handler at wkSend — same as prior captureWebkitHandlers behavior. |
messaging/lib/webkit.js |
110–129 | info | subscribe() still reads ensureNavigatorDuckDuckGo().messageHandlers at subscription time (unchanged, out of scope). |
No security errors, warnings, or critical findings.
Risk Level
Medium Risk — This changes messaging transport initialization timing, which the rubric flags as transport-layer work, but the send path, trust boundaries, origin validation, and page-visible API contracts are unchanged; the diff is a well-tested relocation of an existing snapshot with preserved hardening.
Recommendations
- (info) Consider a brief JSDoc note on
WebkitMessagingConfigthat consumers must construct it at bootstrap (before featureinit()), not lazily alongside transport — the snapshot contract is now implicit. - (info) Optional unit test:
capturedHandlers === null→ transport constructor throwsMissingHandlerwith the new message string. - (info) No additional integration coverage required; the new regression test accurately models the production lazy-transport gap.
Verdict: Approve from a web-compat and security perspective. The change correctly fixes the bootstrap-vs-init race while retaining existing transport hardening.
Sent by Cursor Automation: Web compat and sec
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Asana Task/Github Issue: https://app.asana.com/1/137249556945/task/1212684734587032?focus=true
Description
WebkitMessagingTransport previously captured handler references in its own constructor. Since
ContentFeature.messagingis lazy, transport construction can happen after ContentFeatureinithas run, which includes afterapiManipulationhas applied any prototype-side messageHandlers-nullifying patches.This PR moves the snapshot into WebkitMessagingConfig's constructor. Entry points construct the config eagerly at C-S-S bootstrap, before feature init runs, so the snapshot pre-dates any in-C-S-S apiManipulation patch. Transport constructor reads config.capturedHandlers rather than re-reading window.webkit.messageHandlers.
Adds a regression test: construct config, nullify
window.webkit.messageHandlers, construct transport, send message — send goes through the snapshot and reaches the original handler.
Testing Steps
Checklist
Please tick all that apply: